Skip to content

feat: upgrade to k8s client 5.0 to simplify CR handling #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 23 commits into from
Closed

feat: upgrade to k8s client 5.0 to simplify CR handling #229

wants to merge 23 commits into from

Conversation

metacosm
Copy link
Collaborator

No description provided.

This was referenced Nov 18, 2020
Merged
Closed
@metacosm metacosm closed this Nov 19, 2020
@metacosm metacosm reopened this Nov 19, 2020
@adam-sandor
Copy link
Collaborator

Wow.. this is awesome, I didn't even think of jumping on this yet and now we have a PR :)

@metacosm metacosm closed this Nov 19, 2020
@metacosm metacosm reopened this Nov 19, 2020
@csviri
Copy link
Collaborator

csviri commented Nov 19, 2020

Looks good to me!
Just we should take a look why the integration tests are failing.

@metacosm
Copy link
Collaborator Author

Looks good to me!
Just we should take a look why the integration tests are failing.

Yes, I'm looking into it and have found an issue with the kube client implementation which we are working to fix as well 😄

@metacosm
Copy link
Collaborator Author

That said the current IT failures look scary: VM crash! 😱

@kirek007
Copy link
Contributor

That said the current IT failures look scary: VM crash! 😱

Yep, looks like crash comes from kubrenetes-client internals, something related to resource watch feature

@kirek007
Copy link
Contributor

kirek007 commented Nov 19, 2020 •
edited
Loading

@metacosm I've found that kubernetes client is calling Watch.onClose which is delegated to our code here:
java-operator-sdk/operator-framework/src/main/java/io/javaoperatorsdk/operator/processing/EventScheduler.java line 148.
Removing system.exit(1) line allows to get more understandable error message :)

@metacosm
Copy link
Collaborator Author

Yep been looking at it all morning, trying to find a solution… 😥

@metacosm metacosm force-pushed the improved-k8s-client branch from dafd71c to 407f1ba Compare November 19, 2020 15:42
@metacosm metacosm closed this Nov 19, 2020
@metacosm metacosm reopened this Nov 19, 2020
@kirek007 kirek007 mentioned this pull request Nov 20, 2020
Merged
@metacosm metacosm force-pushed the improved-k8s-client branch 4 times, most recently from 5a0b163 to 930725a Compare November 21, 2020 22:13
@metacosm metacosm changed the title feat: upgrade to k8s client 5.0 to simplify CR handling WIP: feat: upgrade to k8s client 5.0 to simplify CR handling Nov 24, 2020
@metacosm metacosm changed the title WIP: feat: upgrade to k8s client 5.0 to simplify CR handling feat: upgrade to k8s client 5.0 to simplify CR handling Nov 24, 2020
@metacosm metacosm marked this pull request as draft November 24, 2020 09:10
@metacosm metacosm marked this pull request as ready for review November 24, 2020 09:46
@metacosm
Copy link
Collaborator Author

Ready for review @adam-sandor @csviri

@kirek007
Copy link
Contributor

@metacosm thx for PR, I'll going to review it

It would be great to release this as alpha of our sdk
@adam-sandor @csviri.

kirek007
kirek007 reviewed Nov 24, 2020
View reviewed changes
@metacosm
fix: use CustomResourceList as list type
fa2c290
@metacosm
fix: use CustomResourceList as list type
f292b49
@metacosm
fix: use -e flag to get more exception details
de8120a
@metacosm
fix: CRs return their api version string for client to work correctly
9fbad96
@metacosm
fix: finalizer name to follow proper format
e1464a6
@metacosm
fix: use proper kind
94a1251
@metacosm
fix: use proper kind
d6cd795
@metacosm
fix: CRs need to be marked as Namespaced to retrieve a NS-scoped client
046fd30
@metacosm
fix: restore missing getApiVersion
1f113c2
@metacosm
fix: improper plural in CRD
86261be
@metacosm
fix: use the controller's class canonical name as default finalizer name
0ac9ea7
@metacosm
fix: use proper domain name for finalizer prefix
76955ed
@metacosm
fix: unit test
4c58470
@metacosm
fix: actually create a valid finalizer name and check it is
b95956e
@metacosm
fix: set finalizer name instead of using default to match expectations
858ac6e
@metacosm
chore: remove commons-lang3 dependency
e18a90b
@metacosm
chore: replace deprecated cascading by withPropagationPolicy call
126bd86
@metacosm
feat: update kube client to 5.0.0-alpha-2
18735e8
@metacosm
fix: use new finalizer generation logic
8ec63d3
@metacosm
refactor: isolate finalizer validation until HasMetadata provides it
39dd757 
See fabric8io/kubernetes-client#2628
@metacosm
fix: add .io suffix to sample.javaoperatorsdk group
fa9f9b8 
The intent here is to use better groups for example CRDs.
@metacosm metacosm force-pushed the improved-k8s-client branch from 8db7f99 to fa9f9b8 Compare November 25, 2020 12:47
@kirek007 kirek007 mentioned this pull request Nov 25, 2020
Closed
@kirek007 kirek007 changed the base branch from master to feat-update-kubernetes-client-5.0 November 30, 2020 14:34
@kirek007
Copy link
Contributor

I've change merge target to the dedicated branch where we'll track update of kubernetes client. We could release test versions from this branch if needed.

@metacosm
Copy link
Collaborator Author

metacosm commented Jan 7, 2021

I'm keeping this one opened until I can finish #288 which will supersede this PR.

@metacosm
Copy link
Collaborator Author

This has been implemented in #288

@metacosm metacosm closed this Jan 19, 2021
@metacosm metacosm deleted the improved-k8s-client branch January 19, 2021 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s-soroosh s-soroosh s-soroosh left review comments

@kirek007 kirek007 kirek007 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@metacosm @adam-sandor @csviri @kirek007 @s-soroosh